feat: add responsive left sidebar navigation#17
Conversation
Adds 4 question card components to the home page to create vertical scroll content for testing sticky behaviour of the future left sidebar and rail.
Navigation:
- Add LeftSidebar: sticky sidebar with 500ms width transition
- NavLink gains `variant` prop — "rail" (icon-only) vs "mobile" (full)
- Avatar relocated from navbar to sidebar bottom (signed-in users)
- MobileNav: `modal={false}` + custom overlay so Clerk popups work
Responsive Behaviour:
- 📱 < 640px: hamburger → slide-out drawer
- 📲 640–1023px: sticky icon rail (4rem)
- 🖥️ 1024px+: full sidebar with labels (14rem)
Layout:
- Root layout now flex container: Navbar + LeftSidebar + Main
- Sidebar uses `sticky` (not fixed) — scrolls with content, no z-index wars
Tests:
- Split by context: authenticated.desktop, authenticated.mobile, navigation
- Add 2FA/OTP handling (email verification + TOTP)
- Cover "Manage account" modal open/close, overlay tap-to-dismiss
Housekeeping:
- Rename constants.ts → nav-links.constants.ts
- Route type safety via `Route` literal in nav-links
- Add --overlay CSS var, group non-shadcn variables
- Bump @clerk/nextjs 6.36.5, next 16.1.0
Desktop users get persistent navigation without hamburger menus. Mobile keeps
its touch-friendly drawer. Sidebar width animates smoothly between rail and
full modes.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
State Management: - Add SidebarProvider with React Context and localStorage persistence - Create useSidebar hook for consuming collapsed state UI Components: - Add SidebarToggle button with ChevronsLeft/ChevronsRight icons - Circular button with muted background when expanded, ghost when collapsed - Hydration-safe with mounted guard pattern Integration: - Wrap root layout with SidebarProvider - Replace CSS breakpoint-driven width with state-driven classes in LeftSidebar - Update NavLink to use isCollapsed state instead of lg: breakpoints Allows users to manually collapse the sidebar to rail mode (icon-only) or expand to full mode (icon + labels). State persists across navigation and browser sessions via localStorage. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Left Sidebar: - Centre nav items when collapsed to fix icon alignment in rail mode - Inline width constants (used only once) Nav Link: - Apply gap conditionally per variant to fix spacing in icon-only mode - Remove redundant p-0 (size-10 already constrains dimensions) Sidebar Toggle: - Use cn() utility for cleaner conditional class logic Sidebar Provider: - Default to collapsed on screens <1024px, expanded on larger displays - User preference in localStorage still takes priority - Remove unnecessary typeof window check (already in useEffect) Fixes visual alignment bugs in collapsed rail mode and adds responsive UX so tablets/small laptops default to collapsed sidebar while preserving user choice. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughRoot layout now uses a SidebarProvider and includes a persistent left sidebar plus Navbar; Nav components were refactored for mobile/rail variants, globals.css tokens reorganised, new sidebar UI content added to the home page, Clerk appearance tweaked, new/changed Playwright E2E tests added, and linting/config updates applied. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as App Root
participant SidebarProv as SidebarProvider
participant Storage as localStorage
User->>App: Load application
App->>SidebarProv: Mount provider
SidebarProv->>Storage: Read "devflow-sidebar-collapsed"
alt saved state present
Storage-->>SidebarProv: return saved state
else no saved state
SidebarProv->>SidebarProv: check window.innerWidth >= 1024px?
alt >= 1024px
SidebarProv-->>App: init expanded (isCollapsed=false)
else < 1024px
SidebarProv-->>App: init collapsed (isCollapsed=true)
end
end
Note over SidebarProv,App: Context exposes isCollapsed + toggle()
User->>App: Click SidebarToggle
App->>SidebarProv: call toggle()
SidebarProv->>SidebarProv: flip isCollapsed
SidebarProv->>Storage: persist new state
SidebarProv-->>App: context updates
App->>User: LeftSidebar re-renders (rail width changes)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (22)
app/(root)/(with-right-sidebar)/page.tsxapp/(root)/layout.tsxapp/globals.cssbiome.jsoncomponents/clerk-provider.tsxcomponents/navigation/full-logo.tsxcomponents/navigation/left-sidebar.tsxcomponents/navigation/mobile-nav.tsxcomponents/navigation/nav-link.tsxcomponents/navigation/nav-links.constants.tscomponents/navigation/navbar.tsxcomponents/navigation/sidebar-toggle.tsxcomponents/sidebar-provider.tsxe2e/auth.spec.tse2e/authenticated.desktop.spec.tse2e/authenticated.mobile.spec.tse2e/mobile-nav-user-flow.spec.tse2e/navigation.spec.tse2e/smoke.spec.tspackage.jsonsidebar.mdx_docs/mine/nav_mobile_sidebar-and-rail.md
💤 Files with no reviewable changes (2)
- e2e/auth.spec.ts
- e2e/mobile-nav-user-flow.spec.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use British English throughout the project
Files:
components/navigation/left-sidebar.tsxcomponents/navigation/sidebar-toggle.tsxe2e/authenticated.desktop.spec.tscomponents/clerk-provider.tsxe2e/authenticated.mobile.spec.tssidebar.mde2e/navigation.spec.tscomponents/navigation/mobile-nav.tsxcomponents/navigation/full-logo.tsxapp/(root)/layout.tsxcomponents/navigation/nav-link.tsxcomponents/sidebar-provider.tsxapp/(root)/(with-right-sidebar)/page.tsxcomponents/navigation/nav-links.constants.tsx_docs/mine/nav_mobile_sidebar-and-rail.mdcomponents/navigation/navbar.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Only add 'use client' directive when interactivity is needed
Avoid manual useMemo/useCallback unless profiling shows need
Always use @/ import aliases, even for sibling imports (e.g., @/app/fonts not ./fonts)
Files:
components/navigation/left-sidebar.tsxcomponents/navigation/sidebar-toggle.tsxe2e/authenticated.desktop.spec.tscomponents/clerk-provider.tsxe2e/authenticated.mobile.spec.tse2e/navigation.spec.tscomponents/navigation/mobile-nav.tsxcomponents/navigation/full-logo.tsxapp/(root)/layout.tsxcomponents/navigation/nav-link.tsxcomponents/sidebar-provider.tsxapp/(root)/(with-right-sidebar)/page.tsxcomponents/navigation/nav-links.constants.tscomponents/navigation/navbar.tsx
components/clerk-provider.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Implement ClerkProvider in components/clerk-provider.tsx applying shadcn theme and Inter font
Files:
components/clerk-provider.tsx
**/*.{css,postcss}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Tailwind v4 @import syntax: @import "tailwindcss" (not @tailwind directives)
Files:
app/globals.css
app/**/page.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Next.js 16: Dynamic route params is a Promise - must await: { params }: { params: Promise<{ id: string }> }
Files:
app/(root)/(with-right-sidebar)/page.tsx
🧠 Learnings (9)
📚 Learning: 2025-12-10T20:20:46.607Z
Learnt from: michellepace
Repo: michellepace/devflow PR: 7
File: components/navigation/navbar/index.tsx:1-12
Timestamp: 2025-12-10T20:20:46.607Z
Learning: Clerk's Next.js components (SignedIn, SignedOut, SignInButton, SignUpButton, UserButton) from clerk/nextjs can be used inside Server Components without adding 'use client' in the consuming component. They manage client/server boundary internally. When reviewing code, prefer omitting 'use client' in server components that render these Clerk components and avoid introducing client directives solely for these components. This guideline helps maintain server/server boundary and reduce client bundle size.
Applied to files:
components/navigation/left-sidebar.tsxcomponents/navigation/sidebar-toggle.tsxcomponents/clerk-provider.tsxcomponents/navigation/mobile-nav.tsxcomponents/navigation/full-logo.tsxapp/(root)/layout.tsxcomponents/navigation/nav-link.tsxcomponents/sidebar-provider.tsxapp/(root)/(with-right-sidebar)/page.tsxcomponents/navigation/navbar.tsx
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Applies to components/auth/clerk-signin.tsx : Sign In component (components/auth/clerk-signin.tsx) should be a client component with theme-aware logo
Applied to files:
components/navigation/left-sidebar.tsxcomponents/navigation/sidebar-toggle.tsxcomponents/clerk-provider.tsxcomponents/navigation/mobile-nav.tsxcomponents/navigation/full-logo.tsxcomponents/navigation/navbar.tsx
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Use Vitest for unit testing and Playwright for E2E testing
Applied to files:
e2e/authenticated.desktop.spec.tse2e/authenticated.mobile.spec.tse2e/navigation.spec.ts
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Applies to components/clerk-provider.tsx : Implement ClerkProvider in components/clerk-provider.tsx applying shadcn theme and Inter font
Applied to files:
components/clerk-provider.tsxcomponents/navigation/navbar.tsx
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Applies to components/auth/clerk-signup.tsx : Sign Up component (components/auth/clerk-signup.tsx) should use static logo
Applied to files:
components/clerk-provider.tsxcomponents/navigation/mobile-nav.tsxcomponents/navigation/full-logo.tsxcomponents/navigation/navbar.tsx
📚 Learning: 2025-12-10T20:20:54.402Z
Learnt from: michellepace
Repo: michellepace/devflow PR: 7
File: components/navigation/navbar/index.tsx:1-12
Timestamp: 2025-12-10T20:20:54.402Z
Learning: Clerk's Next.js components (SignedIn, SignedOut, SignInButton, SignUpButton, UserButton) exported from clerk/nextjs are designed to work in Server Components without requiring a "use client" directive in the consuming component, as they handle the client/server boundary internally with their own directives.
Applied to files:
package.json
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Use Biome for linting and formatting (replaces ESLint and Prettier)
Applied to files:
biome.json
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Applies to app/(auth)/**/*.{ts,tsx} : Create authentication routes at app/(auth)/sign-in/[[...sign-in]] and app/(auth)/sign-up/[[...sign-up]]
Applied to files:
components/navigation/nav-links.constants.tscomponents/navigation/navbar.tsx
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Applies to app/**/page.{ts,tsx} : Next.js 16: Dynamic route params is a Promise - must await: { params }: { params: Promise<{ id: string }> }
Applied to files:
components/navigation/nav-links.constants.ts
🧬 Code graph analysis (5)
components/navigation/sidebar-toggle.tsx (3)
components/sidebar-provider.tsx (1)
useSidebar(48-54)components/ui/button.tsx (1)
Button(65-65)lib/utils.ts (1)
cn(4-6)
components/navigation/mobile-nav.tsx (5)
components/ui/sheet.tsx (6)
Sheet(131-131)SheetTrigger(132-132)SheetContent(134-134)SheetTitle(137-137)SheetDescription(138-138)SheetClose(133-133)components/ui/button.tsx (1)
Button(65-65)components/navigation/full-logo.tsx (1)
ThemeLogo(15-27)components/navigation/nav-links.constants.ts (2)
NAV_LINKS(9-20)NavLink(3-7)components/navigation/nav-link.tsx (1)
NavLink(20-63)
app/(root)/layout.tsx (2)
components/sidebar-provider.tsx (1)
SidebarProvider(15-46)components/navigation/left-sidebar.tsx (1)
LeftSidebar(13-59)
components/navigation/nav-link.tsx (3)
components/navigation/nav-links.constants.ts (1)
NavLink(3-7)components/sidebar-provider.tsx (1)
useSidebar(48-54)lib/utils.ts (1)
cn(4-6)
components/navigation/nav-links.constants.ts (1)
components/navigation/nav-link.tsx (1)
NavLink(20-63)
🪛 LanguageTool
sidebar.md
[style] ~64-~64: Would you like to use the Oxford spelling “centralized”? The spelling ‘centralised’ is also correct.
Context: ...e - mirror mobile-nav.tsx - Styling - centralised theming like mobile-nav.tsx EXCEPT us...
(OXFORD_SPELLING_Z_NOT_S)
[grammar] ~105-~105: The verb ‘grow’ seems to be in the wrong form here.
Context: ...s.com/docs/flex) | flex-1 | Shorthand for grow + shrink + basis together |
(FOR_VB)
x_docs/mine/nav_mobile_sidebar-and-rail.md
[style] ~254-~254: In contexts where ‘if’ is followed by ‘or’, using ‘whether’ may be more appropriate (and formal).
Context: ...ainer agnostic** | NavLink doesn't know if it's in a Sheet or sidebar | | **Auth f...
(IF_WHETHER)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run E2E Tests
🔇 Additional comments (32)
components/navigation/full-logo.tsx (1)
7-9: LGTM! Reasonable housekeeping change.The change from
interfacetotypefor this simple props type is technically sound and follows common TypeScript conventions for single-property object shapes. Type aliases are often preferred for simple, closed object types like this one.biome.json (1)
34-41: No changes needed—no existing interfaces to conflict with this rule.The codebase contains zero interface declarations across 43 TypeScript files. This rule will only affect new code going forward by enforcing
typeoverinterfacefor consistency. The original concern about breaking existing code with legitimate interface usage patterns does not apply here.Likely an incorrect or invalid review comment.
components/clerk-provider.tsx (1)
24-24: LGTM!The addition of
avatarBox: "size-8"provides consistent avatar sizing across Clerk components, aligning with the PR's goal of moving the UserButton to the sidebar. This follows the existing appearance configuration pattern.e2e/navigation.spec.ts (2)
1-21: Well-structured desktop navigation tests.Good use of role-based selectors and clear skip reasoning for WebKit. The loop through
NAV_LINKSensures all navigation items are tested consistently.
23-73: Comprehensive mobile navigation coverage.The tests cover the key mobile menu interactions: navigation, close button, and overlay dismiss. The
force: trueoption on the overlay click is appropriately documented.One consideration: the tests assume "Home" exists in
NAV_LINKS. This is fine given the current data, but ifNAV_LINKSwere ever reordered or "Home" removed, these tests would fail. You might consider usingNAV_LINKS[0].labelfor consistency, though the explicit "Home" is more readable.components/navigation/nav-links.constants.ts (2)
9-20: Effective use ofas const satisfiespattern.The
as const satisfies readonly NavLink[]pattern preserves literal types whilst ensuring the array conforms to theNavLink[]type. This enables both type safety and precise type inference for consumers.
1-7: Good use of type-safe routing.Importing
Routefromnextprovides compile-time validation for navigation routes. The type definition is clean and well-structured.components/navigation/navbar.tsx (3)
1-6: Clean Server Component implementation.Correctly removes
SignedInandUserButtonimports as the avatar is now in the sidebar. The file remains a Server Component since Clerk components handle the client boundary internally. Based on learnings, this is the preferred approach.
16-21: Appropriate use of native<img>for SVG.The biome-ignore comment correctly explains that SVG logos don't benefit from
next/imageoptimisation (no resizing, format conversion, etc.). This is a valid performance consideration.
34-44: Well-structured conditional authentication display.The
SignedOutwrapper correctly shows sign-in/sign-up buttons only when the user is not authenticated. Thehidden sm:flexpattern ensures these are only visible on desktop, with mobile auth handled byMobileNav.sidebar.md (1)
1-110: Comprehensive sidebar documentation.This documentation provides excellent context for the sidebar implementation, including visual references, breakpoint analysis, and Tailwind utilities guidance. The structure is clear and well-organised.
Regarding the static analysis hints:
- "centralised" (line 64): This is correct British English spelling as per coding guidelines. The Oxford spelling "centralized" is American English.
- "grow" (line 105): The grammar is acceptable in this technical context—it's describing Tailwind's flex shorthand components, not a verb requiring conjugation.
components/sidebar-provider.tsx (2)
15-30: Solid state management with SSR considerations.The implementation correctly handles the SSR/hydration boundary:
- Default state matches server render (expanded)
- localStorage access is properly deferred to
useEffect- User preference takes priority over responsive defaults
This aligns with the PR objectives noting "reduce hydration flash" as a known follow-up item.
48-54: Defensive hook implementation.The
useSidebarhook correctly throws if used outsideSidebarProvider, providing a clear error message for developers. This is a good defensive pattern.components/navigation/nav-link.tsx (4)
6-8: Correct use of @/ import aliases.All imports use the
@/alias as required by the coding guidelines, including sibling imports fromnav-links.constantsandsidebar-provider.
10-18: Well-documented variant prop.The JSDoc comments clearly explain the two variant modes and their intended use cases. This improves code maintainability and developer experience.
27-33: Correct active route detection.The logic
pathname === route || pathname.startsWith(${route}/)correctly handles:
- Exact matches (e.g.,
/for Home)- Nested routes (e.g.,
/community/123matches/community)For the home route
/, the second condition becomesstartsWith("//")which won't spuriously match other routes—so no false positives occur.
35-61: Clean responsive rendering logic.The conditional styling handles both variants elegantly:
- Rail: Switches between icon-only (
size-10) and full width based on collapse state- Mobile: Consistent generous padding for touch targets
The
aria-labelis correctly applied only when showing icon-only, ensuring screen reader users receive the link text. Settingalt=""on the decorative icon with the label providing the accessible name follows best practices.package.json (1)
25-25: Both dependency versions are valid, stable, and free from known vulnerabilities.@clerk/nextjs@6.36.5 and next@16.1.0 are confirmed as stable releases with no known security advisories detected.
app/(root)/layout.tsx (1)
1-19: LGTM!The layout structure correctly wraps content in
SidebarProviderand rendersLeftSidebaralongside the main content. Server Component composition is appropriate here sinceSidebarProviderandLeftSidebarhandle their own client boundaries internally.components/navigation/sidebar-toggle.tsx (1)
1-39: LGTM!The hydration guard pattern correctly prevents SSR mismatches, and the accessibility implementation with
aria-labelandaria-expandedis well done. The icon toggle logic (ChevronsRight when collapsed, ChevronsLeft when expanded) follows intuitive UX conventions.app/(root)/(with-right-sidebar)/page.tsx (1)
29-91: Demo content for testing sticky sidebar behaviour.The question cards provide useful scroll content for validating the sticky sidebar implementation. The end marker clearly indicates the purpose. Consider adding a comment at the top of this section noting that this is temporary test content, or tracking its removal in a follow-up issue if it's not intended for production.
components/navigation/left-sidebar.tsx (1)
1-59: LGTM!The sidebar implementation is well-structured with:
- Correct sticky positioning and dynamic height calculation
- Proper responsive behaviour (hidden on mobile, flex on sm+)
- Smooth width transitions between collapsed (w-16) and expanded (w-56) states
- Good accessibility with
aria-labelon the navigation element- Clean separation of top navigation and bottom user controls
The
'use client'directive is correctly applied due to theuseSidebarhook usage. Based on learnings, the Clerk components (SignedIn,UserButton) work correctly within this client component context.e2e/authenticated.mobile.spec.ts (1)
72-80: Helpful debugging context in assertion message.Good practice including the Sheet
modal={false}hint in the error message — this will aid debugging if Clerk popups are blocked.app/globals.css (2)
53-57: Good organisation of custom theme tokens.Registering
--color-mobile-navand--color-overlayin the@theme inlineblock enables Tailwind to generate the corresponding utilities (e.g.,bg-mobile-nav,bg-overlay), which are used in the mobile navigation overlay.
95-104: Clear separation of non-shadcn variables.The "NOT SHADCN VARIABLES" section clearly delineates custom project tokens from the shadcn-generated ones, improving maintainability.
components/navigation/mobile-nav.tsx (3)
32-44: Good workaround for Clerk popup compatibility.The custom overlay with
modal={false}is a pragmatic solution to allow Clerk modals (which render outside the Sheet portal) to function correctly. The comment explaining the rationale and referencing the E2E test is helpful for future maintainers.
46-54: Dynamic trigger icon and aria-label is well implemented.The toggle between Menu/X icons with corresponding aria-label updates provides clear visual and accessible feedback for the menu state.
86-91: UserButton placement for signed-in users looks good.The
mt-autopositioning correctly pushes the UserButton to the bottom of the sheet, maintaining visual consistency with the signed-out auth buttons placement.x_docs/mine/nav_mobile_sidebar-and-rail.md (4)
11-15: Accurate variant definitions and responsive breakpoints.The overview table clearly maps the three navigation contexts to their respective variants and responsive behaviour. The breakpoint values (sm: 640px, lg: 1024px) align with Tailwind defaults.
127-180: Clear architectural explanation of NavLink component strategy.The component architecture section effectively explains the "container agnostic" pattern and responsive variant handling. The visual ASCII diagram at lines 137–150 and the variants diagram at lines 164–180 clearly illustrate how the same NavLink component is used differently based on container context and breakpoint. The distinction that
railis responsive (handling breakpoint transformation internally via Tailwind) whilstmobileis static is particularly well conveyed.
98-121: Authentication UI section adds helpful clarity.The new "Authentication UI" section (lines 98–121) clearly articulates where auth controls appear across screen sizes and authentication states, which reduces potential confusion for developers implementing or maintaining these components. The tabular format and accompanying flowchart are effective.
235-243: Rationale for nav-links.constants.ts separation is well justified.The explanation correctly identifies the benefits of separating the NAV_LINKS array: single source of truth, multiple consumers, avoidance of "use client" in data, and prevention of circular imports. This is good documentation for maintainability.
E2E Tests: - Require E2E_TEST_EMAIL, E2E_TEST_PASSWORD, and E2E_TEST_OTP env vars - Throw clear error message when credentials are missing - Remove empty string defaults that caused confusing Clerk validation errors Playwright Config: - Enable dotenv to load .env.local for local development - Add dotenv as dev dependency - Use node: protocol for path import per Biome lint rule Style: - Refactor mobile-nav className to use cn() utility Tests now fail fast with an explicit error when env vars are missing, rather than proceeding with empty credentials and producing cryptic Clerk validation errors. dotenv only affects local development — CI/Vercel set env vars directly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
e2e/authenticated.desktop.spec.ts (1)
50-52: OTP input selector could be fragile.Using
page.getByRole("textbox").first()may select unintended elements if Clerk's page structure changes. Consider using a more specific selector if Clerk provides one, such as targeting the OTP input container or using a test ID.e2e/authenticated.mobile.spec.ts (1)
59-61: OTP input selector could be fragile.Using
page.getByRole("textbox").first()may select unintended elements if Clerk's page structure changes. Consider using a more specific selector if available.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
components/navigation/mobile-nav.tsxe2e/authenticated.desktop.spec.tse2e/authenticated.mobile.spec.tspackage.jsonplaywright.config.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use British English throughout the project
Files:
e2e/authenticated.desktop.spec.tscomponents/navigation/mobile-nav.tsxplaywright.config.tse2e/authenticated.mobile.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Only add 'use client' directive when interactivity is needed
Avoid manual useMemo/useCallback unless profiling shows need
Always use @/ import aliases, even for sibling imports (e.g., @/app/fonts not ./fonts)
Files:
e2e/authenticated.desktop.spec.tscomponents/navigation/mobile-nav.tsxplaywright.config.tse2e/authenticated.mobile.spec.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Applies to components/clerk-provider.tsx : Implement ClerkProvider in components/clerk-provider.tsx applying shadcn theme and Inter font
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Use Vitest for unit testing and Playwright for E2E testing
Applied to files:
e2e/authenticated.desktop.spec.ts
📚 Learning: 2025-12-10T20:20:54.402Z
Learnt from: michellepace
Repo: michellepace/devflow PR: 7
File: components/navigation/navbar/index.tsx:1-12
Timestamp: 2025-12-10T20:20:54.402Z
Learning: Clerk's Next.js components (SignedIn, SignedOut, SignInButton, SignUpButton, UserButton) exported from clerk/nextjs are designed to work in Server Components without requiring a "use client" directive in the consuming component, as they handle the client/server boundary internally with their own directives.
Applied to files:
package.json
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Use Next.js 16 with App Router (not Pages Router)
Applied to files:
package.json
📚 Learning: 2025-12-10T20:20:46.607Z
Learnt from: michellepace
Repo: michellepace/devflow PR: 7
File: components/navigation/navbar/index.tsx:1-12
Timestamp: 2025-12-10T20:20:46.607Z
Learning: Clerk's Next.js components (SignedIn, SignedOut, SignInButton, SignUpButton, UserButton) from clerk/nextjs can be used inside Server Components without adding 'use client' in the consuming component. They manage client/server boundary internally. When reviewing code, prefer omitting 'use client' in server components that render these Clerk components and avoid introducing client directives solely for these components. This guideline helps maintain server/server boundary and reduce client bundle size.
Applied to files:
components/navigation/mobile-nav.tsx
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Applies to components/auth/clerk-signin.tsx : Sign In component (components/auth/clerk-signin.tsx) should be a client component with theme-aware logo
Applied to files:
components/navigation/mobile-nav.tsx
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Applies to components/auth/clerk-signup.tsx : Sign Up component (components/auth/clerk-signup.tsx) should use static logo
Applied to files:
components/navigation/mobile-nav.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run E2E Tests
🔇 Additional comments (7)
playwright.config.ts (1)
1-10: LGTM! Dotenv integration correctly implemented.The use of
node:pathprotocol anddotenv.config()for loading local environment variables is correctly implemented. The comments clearly explain the behaviour in different environments (local development vs CI/Vercel).e2e/authenticated.desktop.spec.ts (1)
4-12: Excellent! Environment variable validation addresses past review concern.The explicit validation of required environment variables with a clear error message prevents silent test failures. This directly addresses the previous review comment about empty string defaults.
e2e/authenticated.mobile.spec.ts (2)
4-12: Excellent! Environment variable validation addresses past review concern.Consistent with the desktop test, this validation prevents silent failures when credentials are missing. This directly addresses the previous review comment.
78-86: Helpful documentation for Clerk modal integration.The comment on lines 79-86 clearly explains the relationship between
Sheet modal={false}and Clerk popup compatibility, which will aid future maintainers troubleshooting interaction issues.components/navigation/mobile-nav.tsx (3)
59-62: Excellent!cn()utility usage addresses past review concern.The refactor to use the
cn()utility for className composition provides consistency with the codebase and safer conditional concatenation. This directly addresses the previous review comment.
33-46: Custom overlay implementation is a sound workaround.The custom overlay with
modal={false}on the Sheet is necessary to allow Clerk popups to render correctly outside the Sheet's portal. The comments clearly document this technical constraint and the related E2E test considerations. The overlay maintains tap-to-dismiss functionality and appropriate accessibility attributes.
91-95: UserButton and auth flow integration looks correct.The refactor to use
UserButtonin theSignedInblock and theonClickhandlers on Sign In/Sign Up buttons to close the menu maintains proper user flow. Per learnings, Clerk components handle the client/server boundary internally, so the'use client'directive on this component is correctly applied for theuseStatehook rather than for the Clerk components themselves.Also applies to: 98-123
Dependencies: - next 16.1.0 → 16.1.1, lucide-react 0.561.0 → 0.562.0 - @clerk/themes 2.4.43 → 2.4.46, @clerk/testing 1.13.22 → 1.13.26 Dev Dependencies: - @biomejs/biome 2.3.8 → 2.3.10, vitest 4.0.15 → 4.0.16 - lefthook 2.0.9 → 2.0.12, @types/node 25.0.2 → 25.0.3 - @testing-library/react 16.3.0 → 16.3.1 - vite-tsconfig-paths 6.0.0 → 6.0.3 - baseline-browser-mapping 2.9.5 → 2.9.11 Routine dependency maintenance to keep packages current with security patches and bug fixes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
E2E Testing: - Add e2e/viewports.ts with Tailwind-aligned breakpoints (MOBILE to XXL) - Replace devices["iPhone 12"] with VIEWPORTS.MOBILE across test files - Remove WebKit skip directives — tests now run on all browsers - Remove hardcoded timeouts in favour of Playwright defaults Playwright Config: - Separate browser selection from viewport testing (browserName only) - Re-enable webkit project; keep firefox commented for faster runs - Set open: "never" for HTML report to avoid blocking terminal Developer Experience: - Move Next.js dev indicator to bottom-right (avoids left sidebar overlap) - Add CI=true to lefthook pre-push for CI parity - Update Biome schema to 2.3.10 Viewport definitions now match Tailwind breakpoints exactly, making responsive testing explicit. Browser and viewport concerns are decoupled — tests specify viewport via test.use(), config specifies browsers. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
x_docs/mine/nav_mobile_sidebar-and-rail.md (1)
254-254: Past review comment addressed.Line 254 now correctly uses "whether" instead of "if" for the formal documentation context.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
biome.jsone2e/authenticated.desktop.spec.tse2e/authenticated.mobile.spec.tse2e/navigation.spec.tse2e/viewports.tslefthook.ymlnext.config.tspackage.jsonplaywright.config.tsx_docs/mine/nav_mobile_sidebar-and-rail.md
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use British English throughout the project
Files:
e2e/authenticated.desktop.spec.tse2e/authenticated.mobile.spec.tsx_docs/mine/nav_mobile_sidebar-and-rail.mdplaywright.config.tse2e/navigation.spec.tsnext.config.tse2e/viewports.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Only add 'use client' directive when interactivity is needed
Avoid manual useMemo/useCallback unless profiling shows need
Always use @/ import aliases, even for sibling imports (e.g., @/app/fonts not ./fonts)
Files:
e2e/authenticated.desktop.spec.tse2e/authenticated.mobile.spec.tsplaywright.config.tse2e/navigation.spec.tsnext.config.tse2e/viewports.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Applies to components/clerk-provider.tsx : Implement ClerkProvider in components/clerk-provider.tsx applying shadcn theme and Inter font
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Use Biome for linting and formatting (replaces ESLint and Prettier)
Applied to files:
biome.json
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Use Vitest for unit testing and Playwright for E2E testing
Applied to files:
e2e/authenticated.desktop.spec.tslefthook.ymlplaywright.config.tse2e/navigation.spec.ts
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Use Lefthook for Git hooks: pre-commit runs lint, typecheck, and unit tests; pre-push runs E2E tests
Applied to files:
lefthook.yml
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Enable React Compiler for automatic optimisations
Applied to files:
next.config.ts
🧬 Code graph analysis (2)
e2e/authenticated.desktop.spec.ts (1)
e2e/viewports.ts (1)
VIEWPORTS(11-29)
e2e/authenticated.mobile.spec.ts (1)
e2e/viewports.ts (1)
VIEWPORTS(11-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run E2E Tests
🔇 Additional comments (17)
lefthook.yml (1)
52-53: The Playwright configuration correctly respects the CI environment variable.The comments in lefthook.yml at lines 52–53 are accurate. The playwright.config.ts file explicitly checks
process.env.CIand configures the webServer to runnpm start(production build) in CI mode andnpm run devin local development (line 94). The comment correctly describes this behaviour and the rationale for avoiding dev overlay issues.next.config.ts (1)
6-9: No issues found. ThedevIndicators.positionconfiguration is a valid, documented option in Next.js 16.1, with'bottom-right'being one of the allowed values alongside'bottom-left','top-right', and'top-left'. The implementation correctly positions the development indicator away from the new left sidebar.biome.json (1)
2-2: Schema version 2.3.10 is valid and available. Verification shows this is a standard patch release with bug fixes for various linter rules.e2e/navigation.spec.ts (3)
1-18: LGTM! Well-structured desktop navigation test.The test correctly uses shared viewport presets and NAV_LINKS constants, ensuring consistency with the component implementation. The iteration pattern efficiently covers all navigation links.
20-43: LGTM! Comprehensive mobile navigation flow.The test properly verifies the full mobile navigation cycle: opening the menu, checking link visibility, navigating, and confirming menu closure via
SheetClose. Good coverage of the mobile UX.
45-70: LGTM! Good coverage of menu dismissal behaviours.The tests for both the close button and overlay dismissal are well-structured. The
force: trueoption on line 68 is appropriately documented to handle pointer event interception.e2e/authenticated.desktop.spec.ts (2)
5-13: LGTM! Proper environment validation implemented.The early throw for missing environment variables provides clear feedback when credentials are not configured, preventing confusing failures during Clerk authentication.
18-75: LGTM! Comprehensive desktop authentication flow.The test covers the complete user journey: sign-in with OTP/2FA handling, account management modal interaction, and sign-out verification. The use of
or()for handling multiple authentication states (email OTP vs 2FA) is a pragmatic approach.e2e/viewports.ts (1)
1-29: LGTM! Well-designed viewport presets.The viewport constants are well-documented, align with Tailwind CSS breakpoints, and use
as constfor type safety. This centralised approach ensures consistency across E2E tests.package.json (2)
54-54: Good addition of dotenv for local environment loading.The
dotenvdevDependency supports the updated Playwright configuration for loading.env.localduring local development and testing.
34-34: Next.js 16.1.1 includes the security patch for CVE-2025-66478.The critical RCE vulnerability (CVE-2025-66478, CVSS 10.0) affecting Next.js 15.x and 16.x with App Router has been patched in Next.js 16.1.1. The version specified in package.json is safe regarding this known vulnerability.
playwright.config.ts (3)
1-10: LGTM! Clean environment loading setup.The dotenv configuration correctly targets
.env.localfor local development while being a no-op in CI environments where files may not exist.
59-85: LGTM! Clear separation of browser and viewport testing.The refactored project configuration cleanly separates browser compatibility testing (chromium, webkit) from viewport/responsive testing, which is now handled per-test via
test.use(). This is a sensible architectural decision.
91-98: LGTM! Flexible webServer configuration.The conditional webServer setup correctly handles both local testing (starting dev/prod server) and external deployment testing (e.g., Vercel previews) by skipping the local server when
BASE_URLis provided.e2e/authenticated.mobile.spec.ts (2)
5-13: LGTM! Consistent environment validation.The environment variable validation matches the desktop spec, ensuring consistent behaviour across test files.
18-104: LGTM! Thorough mobile authentication flow coverage.The test comprehensively covers the mobile-specific authentication journey through the hamburger menu. The custom assertion messages (lines 83, 102-103) are particularly helpful for debugging failures related to Clerk popup interactions.
x_docs/mine/nav_mobile_sidebar-and-rail.md (1)
1-255: LGTM! Comprehensive and well-structured documentation.The documentation provides excellent clarity on the navigation architecture with clear ASCII diagrams, variant explanations, and component relationships. The separation of concerns between containers and NavLink variants is well-explained.
| "style": { | ||
| "useConsistentTypeDefinitions": { | ||
| "level": "error", | ||
| "options": { | ||
| "style": "type" | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM! Enforcing consistent type definitions.
The addition of the useConsistentTypeDefinitions rule with "style": "type" is appropriate and aligns with the broader PR objective to standardise TypeScript type definitions across the codebase. Setting the level to "error" ensures consistent enforcement.
Note: This rule enforces type aliases over interface declarations. Whilst both are valid TypeScript constructs, type offers more flexibility for unions, intersections, and primitives. This is a sound stylistic choice for consistency.
🤖 Prompt for AI Agents
In biome.json around lines 34 to 41, the new useConsistentTypeDefinitions rule
is already correct but ensure consistency: keep "style": "type" and "level":
"error" as added, remove or update any other Biome/TS lint rules that prefer
interfaces elsewhere (search for conflicting rules like
useConsistentTypeDefinitions or interface-preference in other config files) so
there are no contradictory settings, and run the linter/CI to verify no
violations remain.
1 participant
Summary
variant="rail"for icon-only modeTest plan
npm run check)npm run test:unit)Known follow-up items
role="toolbar"to sidebar controls section for accessibility🤖 Generated with Claude Code